-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add crstrings library #2
Conversation
Let me know if there are other utilities that you wish you had in the standard |
478b47b
to
f49c5dd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @itsbilal, @jbowens, and @RaduBerinde)
crstrings/utils.go
line 73 at r1 (raw file):
// // fmt.Sprintf("%s%s", a, crstrings.PrependIfNotEmpty(", ", b)) func PrependIfNotEmpty(prefix, s string) string {
I understand what this is trying to do, but this sample usage did not help me understand the value.
Was this meant to help with formatting loops in prepending a separator on iterations after the first iteration?
Similar question for AppendIfNotEmpty
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @itsbilal, @jbowens, and @sumeerbhola)
crstrings/utils.go
line 73 at r1 (raw file):
Previously, sumeerbhola wrote…
I understand what this is trying to do, but this sample usage did not help me understand the value.
Was this meant to help with formatting loops in prepending a separator on iterations after the first iteration?Similar question for
AppendIfNotEmpty
.
It's mostly for printf statements. E.g. Sprintf("%s%s %s%s", x, PrependIfNotEmpty(", ", y), z, PrependIfNotEmptry(", ", u)
That would normally take 5+ lines of code.
Previously, RaduBerinde wrote…
Though maybe it would be cleaner to do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed (commit messages unreviewed), 1 unresolved discussion (waiting on @itsbilal, @jbowens, and @RaduBerinde)
crstrings/utils.go
line 73 at r1 (raw file):
Previously, RaduBerinde wrote…
Though maybe it would be cleaner to do
Sprintf("%s%s%s", a, crstrings.If(b != "", ", "), b)
Thanks for the clarification. Either is fine by me.
This library contains some general convenience functions related to strings and string slices.
f49c5dd
to
8212df1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @itsbilal, @jbowens, and @sumeerbhola)
crstrings/utils.go
line 73 at r1 (raw file):
Previously, sumeerbhola wrote…
Thanks for the clarification. Either is fine by me.
I replaced them with a single WithSep(a, separator, b)
function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TFTR!
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @itsbilal, @jbowens, and @sumeerbhola)
This library contains some general convenience functions related to
strings and string slices.
This change is